-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
persistent_dict: make siphash24 optional #250
Conversation
Thanks for the contribution. I can see where you're coming from. At the same time, I'm a bit torn. Suppose a user fills a |
Could you look into the CI failures? |
CI failures were a mindless typo. I don't have a strong defense for this change, so if you think the value of enforcing consistency even as the package graph changes, I'll most likely just package |
I'd be willing to give this a shot. But be warned: If it blows up on me in some way, I'll probably just revert it. Also, Ruff had some more complaints. Could you take a look? |
That seems reasonable. The latest patch should pass all checks. |
Thanks! |
I don't know about this- now users of This affects downstream packages too (see e.g. https://github.com/matthiasdiener/skvlite/actions/runs/10066243771/job/27827181383?pr=13, which tests the size of a PersistentDict-based sqlite file, where the size now depends on whether |
To be fair, the size of that SQLite file could be considered an implementation detail. 523ff36 rewords the warning so as to be more actionable. |
From #242:
I maintain
pytools
andpyopencl
on Void Linux and, as long as the hash interface is equivalent, makingsiphash24
optional will slightly reduce the maintenance burden of carrying these packages alongside the rest of the numerical stack.